Voice Broadcast - Improve live indicator icon rendering#7579
Voice Broadcast - Improve live indicator icon rendering#7579
Conversation
Florian14
force-pushed
the
feature/fre/voice_broadcast_live_indicator
branch
from
a9b1637 to
288fc35
Compare
Florian14
requested review from
a team and
jmartinesp
and removed request for
a team
|
EDIT: it seems to be a expected behaviour. |
|
Also, it might be a bit subjective, but the debounced clicks on the -30s and +30s buttons might be a bit too 'eager'? I hit the -30s several times trying to go back ~1:30min and it seemed to get 'stuck', I just realised what was happening because I'm familiar with our 'debouncedClicks` function in the codebase, but at first it seemed like some weird bug. |
|
I also found this: The buffering icon appeared although I'm not at the end of the stream - in fact, the audio kept playing. It fixed itself after some time, but maybe there's a missing check for the current position in the stream? |
jmartinesp
left a comment
jmartinesp
left a comment
There was a problem hiding this comment.
Since 2 of the issues I mentioned are not strictly related to the new feature and the other one seems to be expected behaviour, I'd say it LGTM. Just a couple of suggestions.
| private fun updateLiveListeningMode(seekPosition: Int? = null) { | ||
| isLiveListening = when { | ||
| // the current voice broadcast is not live (ended) | ||
| !currentVoiceBroadcastEvent?.isLive.orFalse() -> false |
There was a problem hiding this comment.
Little nit pick here: I'd either use .not() at the end or .isLive == false, the current version makes the starting ! easy to miss.
| val seekDirection = seekPosition.compareTo(getCurrentPlaybackPosition() ?: 0) | ||
| when { | ||
| // backward | ||
| seekDirection < 0 -> false |
There was a problem hiding this comment.
Oh, that's what caused one of the issues I mentioned. I guess it's a expected behaviour then.
| when { | ||
| // backward | ||
| seekDirection < 0 -> false | ||
| // forward: check if new sequence is the last one |
|
@jmartinesp Thanks for the review, I took your feedback into account:
About the buffering, I thought it was fixed but I also reproduced it, I still have an issue related to the chunks recovery in my player, I will consider it in a dedicated bugfix. |
|
SonarCloud Quality Gate failed. |
3 participants
Type of change
Content
Improve the rendering of the "live" indicator icon in the timeline.
Fixed a bug on the playlist recovery which was handling events from a potential previous voice broadcast.
Minor cleanup.
Motivation and context
Continue #7127
The live indicator icon is not updated according to the following rules:
Screenshots / GIFs
voice_broadcast_live_indicator.mp4
Tests
Tested devices
Checklist